Skip to content

refactor: move NullComm from NDSL to pace#168

Merged
oelbert merged 1 commit into
NOAA-GFDL:developfrom
romanc:romanc/move-null-comm
Jan 13, 2026
Merged

refactor: move NullComm from NDSL to pace#168
oelbert merged 1 commit into
NOAA-GFDL:developfrom
romanc:romanc/move-null-comm

Conversation

@romanc
Copy link
Copy Markdown
Collaborator

@romanc romanc commented Jan 6, 2026

Description

We decided to remove NullComm from NDSL in favor of MPIComm and LocalComm, see NOAA-GFDL/NDSL#318 for context. pace exposes a NullCommConfig and is heavily relying on NullComm in tests. We thus suggest to move NullComm to pace/comm.py.

/cc @FlorianDeconinck

How Has This Been Tested?

Did a search and manual replace of NullComm imports. CI is still green. Copied over a simple test from NDSL.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation: N/A
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • New check tests, if applicable, are included

@romanc
refactor: move NullComm from NDSL to pace
8b15674 
We decided to remove `NullComm` from NDSL in favor of `MPIComm` and
`LocalComm`, see NOAA-GFDL/NDSL#318 for context.
pace exposes a `NullCommConfig` and is heavily relying on `NullComm` in
tests. We thus suggest to move `NullComm` to `pace/comm.py`.
@romanc romanc requested review from fmalatino and oelbert January 6, 2026 09:47
@romanc romanc mentioned this pull request Jan 6, 2026
Merged
7 tasks
@romanc
Copy link
Copy Markdown
Collaborator Author

romanc commented Jan 13, 2026

@oelbert I'd like to move on this PR. It is blocking NOAA-GFDL/NDSL#350, which we scheduled for the next NDSL release. Is there anything I can do from my side to get traction on this PR?

To follow up on our quick discussion about NullComm on Friday: I suggest to move NullComm as a first step (to unblock the NDSL release) and then do a follow-up issue in pace to replace its usage. I'm happy to write that issue if that's what's blocking us here ...

@oelbert
Copy link
Copy Markdown
Collaborator

oelbert commented Jan 13, 2026

If you could write a quick issue that'd be great, and I'll queue the merge

@oelbert
Copy link
Copy Markdown
Collaborator

oelbert commented Jan 13, 2026

Though we'll have to actually change it to LocalComm for some of the tests in PyFV3 and PySHiELD but that's lower priority

@romanc romanc mentioned this pull request Jan 13, 2026
Open
@romanc
Copy link
Copy Markdown
Collaborator Author

romanc commented Jan 13, 2026

If you could write a quick issue that'd be great, and I'll queue the merge

Follow-up issue #172. Thanks for hitting all the buttons today 🙏

@oelbert oelbert added this pull request to the merge queue Jan 13, 2026
Merged via the queue into NOAA-GFDL:develop with commit 516427e Jan 13, 2026
2 checks passed
@romanc romanc deleted the romanc/move-null-comm branch January 13, 2026 19:17
@romanc romanc mentioned this pull request Feb 24, 2026
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fmalatino fmalatino fmalatino approved these changes

@oelbert oelbert Awaiting requested review from oelbert

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@romanc @oelbert @fmalatino